Tweak open connection limits for unstaked connections#6953
Tweak open connection limits for unstaked connections#6953lijunwangs wants to merge 8 commits intoanza-xyz:masterfrom
Conversation
lijunwangs
force-pushed
the
adjust_connection_count
branch
from
ae31e3e to
c427776
Compare
lijunwangs
force-pushed
the
adjust_connection_count
branch
2 times, most recently
from
1836c15 to
9aaaea4
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6953 +/- ##
=======================================
Coverage 83.0% 83.0%
=======================================
Files 827 827
Lines 362695 362694 -1
=======================================
+ Hits 301233 301249 +16
+ Misses 61462 61445 -17 🚀 New features to boost your workflow:
|
|
Memory usage: observed using the bench-vote tools. Connections: 500： 241.844 (MB) 2000: 645.684 (MB) |
lijunwangs
force-pushed
the
adjust_connection_count
branch
from
aae6915 to
285de8b
Compare
lijunwangs
force-pushed
the
adjust_connection_count
branch
from
285de8b to
345e3af
Compare
This is for how much RX window and max streams per connection? |
alexpyattaev
left a comment
alexpyattaev
left a comment
There was a problem hiding this comment.
Overall the direction is good, but I think this needs to get narrowed down to only address the unstaked connection limit, rather than everything at once. Especially we must avoid changes to SWQOS quotas.
| }; | ||
|
|
||
| const MAX_UNSTAKED_STREAMS_PERCENT: u64 = 20; | ||
| const MAX_UNSTAKED_STREAMS_PERCENT: u64 = 50; |
There was a problem hiding this comment.
We really should not be changing this here. This is completely unrelated change and will not have the impact you intend.
There was a problem hiding this comment.
We are using the connection count to distribute the streams/s to different connections out of the total streams/s allowed. With increased connection count quadrupled, the per connection TPS for unstaked, can be reduced to 1/4. The increased percentage reduces that effect.
Our allocation of staked vs. unstaked does not reflect the real usage in MB:
There was a problem hiding this comment.
Yes, it does, but it also cuts massively into the staked TPU bandwidth allocations, reducing them. Not sure that is desirable.
There was a problem hiding this comment.
The staked is not using the allocated bandwidth. See the max. Even 50% --> 250K/s should be more than enough for them to use realistically.
| pub const DEFAULT_MAX_UNSTAKED_CONNECTIONS: usize = 2000; | ||
|
|
||
| /// The maximum number of connections that can be opened for unstaked peers. | ||
| pub const DEFAULT_MAX_UNSTAKED_CONNECTIONS_PER_IPADDR: usize = 4; |
There was a problem hiding this comment.
Should this change be in a separate PR?
lijunwangs
force-pushed
the
adjust_connection_count
branch
from
345e3af to
471845d
Compare
|
this seems to be making several changes that are unrelated, but on the path to a singular goal. please split this up to isolate them |
lijunwangs
force-pushed
the
adjust_connection_count
branch
from
471845d to
6c35787
Compare
4 participants
Problem
There are only 500 unstaked connection while we allow per address 8 concurrent connections. This leads to large number of connections from unstaked clients to contend for 500/8 for about 62 slots and cause churning.s
Summary of Changes
Reduce the per-connection from unstaked clients to 4 per ip address
Increase total connection count for unstaked connections to 2000.
Classifying eviction metrics between staked vs non-staked.
Fixes #